Skip to content

Fixup #331

Merged
merged 3 commits into from
Jul 5, 2023
Merged

Fixup #331

merged 3 commits into from
Jul 5, 2023

Conversation

donald
Copy link
Collaborator

@donald donald commented Jul 5, 2023

Fix up some errors of #328 and #326:

  • Don't start lightdm on servers
  • Keep user informed while update is in progress
  • run async pdist as a service to capture status and output

After the condition on the lightdm tag was removed, the display manager
would start on servers, too. Add condition on the desktop tag.
Copy link
Contributor

@pmenzel pmenzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two times one space too much, but very minor nits.

avoid the error message

"Update and reboot required"

Rest looks good.

Shouldn’t we add an explicit After=startup-tags.service to lightdm.service for correctness despite it is correctly ordered already due to Before=sysinit.target? But that could change?

A synchronous pdist may take some time without anything happening on
the console. At the same time, other messages from systemd might be
emitted, scrolling the original "Update and reboot required" to a less
prominent place.

Keep sending messages at regular intervals.
Run the update as a separate service instead of as a forked
process to avoid the error message

    pdist-bootcheck.service: Failed to destroy cgroup /system.slice/pdist-bootcheck.service, ignoring: Device or resource busy

and to capture the exist status in systemd and the output in the
journal.
@donald
Copy link
Collaborator Author

donald commented Jul 5, 2023

Shouldn’t we add an explicit After=startup-tags.service to lightdm.service for correctness despite it is correctly ordered already due to Before=sysinit.target? But that could change?

Hmmm. My reasoning as a counter argument:

With the previous changes we make the promise, that normal service units can rely on basic mariux64 features by default without explicit configuration. These are: startup tags available, ip address configured (not necessarily remotes already reachable), automount-path ( /project etc ) working (if local), vlans configured, maybe more. The motivation was to make service configuration simpler to avoid errors.

A normal service (with default dependencies) like lightdm doesn't need to care or enumerate whatever it needs from the set.

I think, its generally preferable, if we have the dependency in one place only (in this case at the provider and with Before=) and not redundantly mirrored on both sides. The promise is "services can use startup tags" but the fact, that there is a service "startup-tags.service" is an implementation detail. If we change the service name to mx-startup-tags.service or put everything into a single synchronous "mx-basic.service" script or reorganize everything to use a target for the combined logic (mx-basic.target), we can just do so without changing anything at the normal service units. This would not be possible, if the details were duplicated into the normal service units.

@donald donald merged commit 21e3111 into master Jul 5, 2023
Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants